I would expect this to be somewhere in the helper, though. So we wouldn't be adding public attributes.
Additionally, I don't fully understand the property patching bit — why is it necessary? Also, do does the connection object even need to be made thread-safe? Why?

I've revamped the whole thing - getting rid of the meta class, decorator, property patching, and avoiding adding any public attributes. See what you think. Great question about thread-safety. I think you are right - I don't think it's needed as the usual pattern involves one connection per thread. If so we can further simplify the SSLConnection class. Let me know if you think we should remove.

@julianz- so I did some research and here's my observations:

1. mass-searching source code, it seems like only CherryPy/Cheroot (often being vendored into something else) have any references to thread-safety: https://grep.app/search?f.lang=Python&regexp=true&q=%28threadsafe|thread-safe|thread+safe%29.*\n.*\n.*SSL\.Connection (CherryPy is what first had this module until the HTTP/WSGI part was split out of it).
2. mature projects like urllib3 that also make use of PyOpenSSL don't have any locking: https://github.com/urllib3/urllib3/blob/b20c836/src/urllib3/contrib/pyopenssl.py (although, it might be interesting to look at how they organize their socket wrapper)
3. somebody asked about thread-safety upstream and Paul said that it's fine: Is the pyopenssl thread-safe? pyca/pyopenssl#424. But he made a remark about subinterpreters / mod_wsgi and I know that CherryPy has some code that is probably not being tested in CI related to mod_wsgi. But this was nearly a decade ago.
4. https://stackoverflow.com/a/61826374/595220 hints that prior to OpenSSL 1.1.0 there would be some problems but not anymore. That version hit EOL 6 years ago: https://endoflife.date/openssl. So we shouldn't need to care about it.
5. Is SSL_SESSION supposed to be thread-safe? openssl/openssl#25343 / https://crypto.stackexchange.com/a/41344/70030 / https://docs.openssl.org/1.0.2/man3/threads/#description claim that low-level SSL_SESSION isn't thread-safe. But given the PyOpenSSL issue, I'm inclined to believe that it's fine in the Python land.
6. I did some Git paleontology and the commit that added locking on Sep 12, 2006 cherrypy/cherrypy@f2e3c1c (f2e3c1c); being terrified of exec(), I rewrote it to a metaclass on Sep 2, 2018 in e9e070b
   That commit does not really explain anything but references pyOpenSSL/tsafe.py. This was pre-PyPI which explains why things were being copied across projects and/or vendored. The first PyOpenSSL release on PyPI was 0.6 on Jun 4, 2008. So it's already two years past the CherryPy commit. We can't say for sure which version this was copied from. The very first import of PyOpenSSL from some other VCS into Git was on Feb 2, 2008 (pyca/pyopenssl@897bc25) but it's tagged as v0.14a1 while is probably an error or some weird historic artifact. However, that commit does contain the exec() snippet with locking that ended up in CherryPy — https://github.com/pyca/pyopenssl/blob/897bc2556fed43b76f6d1b14470c3e806df15af8/tsafe.py. On Jul 28, 2010, that module moved within the repo to https://github.com/pyca/pyopenssl/blob/94cc894795226537daf2f4e79e2d142ff51eacec/OpenSSL/tsafe.py (pyca/pyopenssl@94cc894#diff-eb4dc4606a8f26382d033c9859d6b5be8043e8bc511522f2e014e181bd09c09d). More history of this module: https://github.com/pyca/pyopenssl/commits/e41098f271dc96d6c411172bb11bbe5816f78710/src/OpenSSL/tsafe.py. It was deprecated on Sep 4, 2017 in PyOpenSSL v17.3.0 (Deprecate OpenSSL.tsafe pyca/pyopenssl#655 / Fixes #655 -- deprecate OpenSSL.tsafe  pyca/pyopenssl#673) and removed fully on Nov 27, 2020 in v20.0.0 (Remove deprecated tsafe module. pyca/pyopenssl#913). The maintainers thought nobody uses it and weren't sure it was functional. At the time of deletion, it was still using the original exec() (with an additional method added to the list throughout its life).

So I'd get rid for the hacks and made it as simple as possible, instead of trying to preserve them w/o a good recorded/explainable justification.

Very thorough! But obviously removing the thread-safety is a potentially breaking change if anyone is relying on it.

I wonder if we should have something for sock_shutdown().

Unlike makefile() which I just had to add, the linter is not complaining about this method being missing. We could add for socket compatibility I guess, but it's just going to call the shutdown method. If we try to replicate the whole socket interface then wouldn't we need to add a bunch of other methods? Then we end up adding all kinds of cruft?

I'm mostly curious about the calling code using it rather than us reimplementing it. This class will get the implementation through inheritance.

@julianz- I just remembered that the calling code actually invokes sock_shutdown() and not shutdown(). See

I wonder if we could have interface that would be uniform across adapters and wouldn't have that ugly getattr() hack, which is effectively an abstraction leak.

maybe then we assume all connection adapters (including SSLConnection) implement sock_shutdown?

@julianz- yes, that's sort of what I was considering. But looking at

This means a separate PR as it feels like a significant effort to figure out. If you're up for it, please try composing a patch scoped to implementing this compatibility interface. It would probably need to come before this PR, though.

ok will give it a try

I'm actually not sure if this constitutes a breaking change. SemVer talks about breaking changes in the context of API compatibility and the API seems to have remained intact. WDYT?

If the API offered thread-safety and now suddenly we are saying these methods are not thread-safe that sounds like a breaking change to me, no? It's somewhat moot I guess if, in practice, no-one needs these methods to be thread-safe but just in case it's better to clear this is a potentially breaking change. If not a breaking change what category would be best?

Honestly, I'm torn. We could keep it breaking. Otherwise, it'd probably be misc or something like that, which is not very prominent in the change log.

Sure. That makes sense. I'm fine to move to misc.

@julianz- I'm not opposed to keeping it breaking. Just thinking out loud. If it's breaking, I'd bump the major version component on release. Otherwise, it might be a minor change.

is Breaking Bad? haha. i'm torn also. i changed to misc but if you want to go back to breaking let me know.

Let's keep it misc, I suppose.

Why do we need this? Can we replicate the method signature exactly?

makefile() I added because the linter complained about makefile being abstract and needing an implementation.

Which linter? Do you have logs?

@julianz- I'm pretty sure the abstract method is in the adapter and not in SSL.Connection. Are you sure this is needed?

https://github.com/cherrypy/cheroot/actions/runs/19145536408/job/54722201402?pr=787

I guess it's not really abstract in SSL.Connection - it's just marked as not implemented. The linter interprets that as abstract?

In https://github.com/cherrypy/cheroot/pull/787/files#r2498797283 I explained that this would require more work. Specifically because stdlib's socket.socket does not have this method. Also, this wouldn't need to go into a variable but called directly.